Wave 2b — data adapters (kv · database · prisma-adapter-mysql)#12
Conversation
…a-adapter-mysql) Seed commit to open the Wave 2b draft sub-PR into the Wave 2 integration branch (feat/package-quality-wave2-adapters). Implementation follows the 2b slice plan in .llm/tmp/run/feat-package-quality-wave2-adapters--adapters/plan.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wave 2b generator handoff — data adaptersGenerator work for sub-wave 2b is complete and pushed to Scope completed
Commits
Validation evidence
HandoffReady for separate-session IMPL-EVAL. Do not mark this PR ready until IMPL-EVAL returns PASS. |
|
@copilot |
…ave2b IMPL-EVAL PASS Co-authored-by: rickylabs <129366361+rickylabs@users.noreply.github.com>
IMPL-EVAL — Sub-wave 2b (data adapters) → PASSRan as a separate-session evaluator over Blocker fixed (in
|
| Package | check | doc-lint | publish dry-run (slow types) | tests | README |
|---|---|---|---|---|---|
| kv | ✅ | ✅ 4 files | ✅ 0 | ✅ 78 | 161 |
| database | ✅ | ✅ 10 files | ✅ 0 | ✅ 3 | 249 |
| prisma-adapter-mysql | ✅ | ✅ 1 file | ✅ 0 | ✅ 8 | 302 |
fmt --checkclean (90 files); scoped doctrine--root <pkg>: FAIL=0 for all three (WARN-only residuals: file sizes, one kvdexany, db scriptDeno.exit, dbexport default).- Consumer gate:
sagas/triggers/workers✅. No.tsconsumer of the removed@netscript/database/interfaces; CLI'sPrismaPgcomes from@prisma/adapter-pgdirectly, not the removed database root re-export.
Out-of-scope caveat (not blocking)
packages/cli and plugins/streams check fail on pre-existing isolated-declarations debt in files untouched by 2b (copy-official-plugin.ts:205, probe-context.ts) — both fail identically on base e5d54e2. Same class as the repo-wide arch:check residual already logged in drift.md.
Verdict: PASS. PR left in draft per handoff — ready to mark ready for the umbrella.
Co-authored-by: rickylabs <129366361+rickylabs@users.noreply.github.com>
🤖 Augment PR SummarySummary: Aligns Wave 2b “data adapter” packages ( Changes:
Technical Notes: Publish hygiene is improved via explicit 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| const log = debug | ||
| ? (msg: string, ...args: unknown[]) => console.log(`[${name}] ${msg}`, ...args) | ||
| : () => {}; | ||
| const log = debug ? (_msg: string, ..._args: unknown[]) => {} : () => {}; |
There was a problem hiding this comment.
debug is documented as “Enable debug logging”, but log is currently a no-op even when debug === true, so the flag has no observable effect (and the log(...) calls below will never emit). This looks like an unintended behavior regression vs the option’s contract.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| const status = await adapter.getStatus(); | ||
| assertEquals(status.connected, true); | ||
|
|
||
| const result = await adapter.executeRaw<{ query: string }>('select 1'); |
There was a problem hiding this comment.
runDatabaseAdapterContract asserts executeRaw() returns an object with a query field; real database adapters will typically return driver result sets/rows instead. As written, this shared contract runner likely can’t be reused by downstream adapters beyond the mock implementation.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| */ | ||
| export interface PrismaTracingProvider { | ||
| /** Return an OpenTelemetry-compatible tracer. */ | ||
| getTracer(name: string, version?: string, options?: unknown): unknown; |
There was a problem hiding this comment.
PrismaTracingProvider.getTracer() returns unknown and is then cast to a full TracerProvider later, so an invalid tracer provider can slip through type-checking and fail at runtime inside ActiveTracingHelper. Consider tightening the public type here to match the actual minimum Tracer/TracerProvider shape you rely on.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
@copilot fix issues raised by augment in above comments (if legitimate) then reply back with a description of each fixes you applied |
| for await (const entry of store.list<{ value: number }>({ prefix: ['contract'] })) { | ||
| values.push(entry.value.value); | ||
| } | ||
| assertEquals(values, [1, 2]); |
There was a problem hiding this comment.
store.list(...) ordering isn’t part of the KvStore contract, so assertEquals(values, [1, 2]) can make downstream adapter contract tests flaky if an implementation yields entries in a different order. Consider asserting set membership / sorting before comparing instead of relying on iteration order.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| | 'READ UNCOMMITTED' | ||
| | 'READ COMMITTED' | ||
| | 'REPEATABLE READ' | ||
| | 'SNAPSHOT' |
There was a problem hiding this comment.
PrismaMySqlIsolationLevel includes 'SNAPSHOT', but startTransaction() interpolates the isolation level directly into SET TRANSACTION ISOLATION LEVEL ..., which MySQL doesn’t accept (it will error if Prisma or a caller passes 'SNAPSHOT'). Consider either removing 'SNAPSHOT' from the public type or mapping it to a valid MySQL level (e.g. 'REPEATABLE READ').
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Co-authored-by: rickylabs <129366361+rickylabs@users.noreply.github.com>
Fixed in
Validation: |
|
Use harness Can you study the eventuality of migrating to https://github.com/denodrivers/redis (https://jsr.io/@db/redis) What benefits does it brings beside obvious package size and less npm registry dependencies ? Write a new comment about the verdict with exhaustive deep dive also update drift accordingly |
Co-authored-by: rickylabs <129366361+rickylabs@users.noreply.github.com>
Migration study —
|
| Package | What it uses ioredis for | Command surface |
|---|---|---|
packages/kv (RedisKvAdapter) |
KV + watch | get set psetex del exists mget scan watch/unwatch multi/exec publish + EventEmitter pub/sub (.on('message')), TLS |
packages/queue (redis.adapter) |
work queue | zadd lpush brpoplpush (blocking) zrangebyscore zrem lrem, dual command+blocking clients |
packages/plugin-sagas-core (redis-transport) |
saga transport | Streams consumer-groups: xadd xgroup xreadgroup xack xpending xclaim + .duplicate() |
packages/sdk |
isolation | dynamic import() to keep ioredis out of the SSR module graph |
packages/cli (kernel/constants/windows.ts) |
scaffolding | emits npm:ioredis@^5 into generated import maps |
So a full cut-over is a cross-package, multi-wave effort, which is the main reason it doesn't belong in 2b.
1. Benefits beyond package size / fewer npm deps
Yes — several that align directly with what Wave 2 is for:
- Deno-native, no
npm:/node-compat in the publish graph. ioredis is the one npm specifier left in@netscript/kv's graph. Dropping it moveskv(and eventuallyqueue/sagas-core) toward a pure-JSR dependency closure — exactly the "0 npm registry dependencies" posture Wave 2 is chasing. - First-class JSR types, no
@types/ioredis.@db/redisis TypeScript-first, published on JSR, and passes JSR's no-slow-types gate — so it's--isolatedDeclarations-friendly out of the box (the same gate we've been fighting all of Wave 2). - Async-iterator pub/sub matches our public API. Our
watch()/watchPrefix()already exposeAsyncIterable. Today we hand-roll an EventEmitter→iterator bridge over ioredis's.on('message')(seeredis.adapter.ts:565-633).@db/redisexposesfor await (const { channel, message } of sub.receive())natively, letting us delete that bridge boilerplate. - Native
using/Symbol.dispose— matches our existing[Symbol.asyncDispose]resource pattern. - Deno-Deploy-shaped (TCP via
Deno.connect) — better long-term edge alignment than ioredis's Node socket stack (caveat in §3).
2. Does it cover everything we've implemented?
Commands: full coverage. Verified against source/tests (v0.41.2): basic KV, scan (cursor+MATCH+COUNT), WATCH/MULTI/EXEC optimistic concurrency, Streams consumer-groups (xreadgroup/xack/xpending/xclaim), blocking lists (brpoplpush/blpop), sorted sets, and EVAL. Nothing we use is missing.
Behavioral/API differences that need adapter work (not blockers):
- Pub/sub is async-iterator, not EventEmitter → rewrite kv's
.on('message')handlers asfor awaitloops (net simpler given ourAsyncIterablesurface). - No
.duplicate()→ replace with a secondconnect()for the dedicated subscriber (kv) and saga subscriber connection. scancursor isnumber-in /string-out (#391) → coerce in adapter.- Blocking
xread/xreadgroupreturnsnullon timeout (#333, open) → must guard before destructuring in the saga transport. - Single connection serializes commands (#412) → mitigated already by our
MGETbatching inlist()and queue's separate blocking client; hot paths can usepipeline()/experimental pool if needed. - Connection options mapping:
connectTimeout→signal: () => AbortSignal.timeout(...),maxRetriesPerRequest→maxRetryCount,lazyConnect→createLazyClient;keepAliveinterval has no direct equivalent (onlynoDelay). - No Sentinel (#428) — only matters if prod uses Sentinel HA. We target single-instance Garnet/Redis, so likely n/a — must confirm before committing.
- No
defineCommand(we don't use it;sendCommandexists as fallback).
3. Type-safety / codebase quality?
Net positive:
- Removes the last npm dependency from
kv's publishable closure and removes a@types-shaped surface; first-class JSR types are isolated-declaration clean. - Lets us delete the EventEmitter bridge in favor of native async iterators — less code, fewer footguns.
- Stronger alignment with our
./testingport-contract entrypoint: the contract suite is the right place to pin thescan-cursor,xread-null, and concurrency behaviors so an adapter swap stays observably equivalent.
Risk to weigh: @db/redis still self-labels "experimental" (v1 roadmap #437 open), and the open xread null (#333) and concurrency (#412) issues need adapter-level guards + contract coverage. No Deno-Deploy production story yet (#480).
Recommendation
Open a dedicated migration track (post-Wave 2), gated behind a spike:
- Port
kvfirst (smallest surface, async-iterator-aligned) behind the./testingport-contract entrypoint. - Validate
xreadnull-on-timeout + single-connection concurrency against Garnet. - Confirm no Sentinel dependency in target deployments.
- Then
queue, thensagas-coreStreams; finally updatecliscaffolding import maps.
Out of 2b scope — this is a forward-looking opportunity, recorded in drift.md, not a 2b blocker. 2b's IMPL-EVAL verdict is unchanged (PASS).
2b (kv·database·prisma-adapter-mysql) merged into the umbrella via PR #12 (55f6108) after separate-session IMPL-EVAL PASS. 2c (queue·cron) bootstrapped off the umbrella: worktree + branch + seed run docs (0a4e043), draft PR #13 into the umbrella. Adds the 2b-merge + 2c-bootstrap rows to the base-sync/merge log and records the @db/redis migration as deferred to a future track (NOT Wave 2). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#12) A real Fresh routes/*.tsx handler gated on an active session and an admin role, at sibling auth.md's depth: resolves the session via backend.sessions.getSession, maps to a Principal with principalMapper, and fails closed on both checks. Grounded in packages/auth-better-auth + @netscript/service/auth exports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM
…-tos, auth example (#686) * docs: clarify PORT/typed-config relationship across 8 pages (#660 #7) Aspire injects PORT at runtime; netscript.config.ts services.<name>.port (apps.<name>.port for Fresh apps) is the typed source of truth the scaffold wires as the fallback default. One shared sentence at each first parseInt occurrence so the raw-env read no longer reads as the canonical pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * docs: reference showcase ledes, see-it-live links, streams framing (#660 #8 #9 #10 #15) - reference/ai: explicit note that AI provider keys have no typed config surface; raw Deno.env.get is the supported path (#8). - reference/queue + reference/kv: See-it-live link blocks to the queue-kv-cron how-to and data-persistence concept page (#9). - reference/contracts + reference/sdk: showcase sentence per lede — contracts obviate manual req.json() validation; defineServices() wires the whole typed stack in one call (#10). - durable-workflows/streams: one sentence reframing no in-process subscribe() as an intentional single HTTP/SSE surface (#15). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * docs: add runnable end-to-end blocks to three decision how-tos (#660 #11) discover-services, choose-a-queue-provider, and add-a-task-runtime-adapter each gain one copy-pasteable end-to-end block (resulting file + full command sequence) at deploy-local-aspire density, demonstrating the pluggability the pages previously only asserted. All commands/APIs grounded in package source. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * docs: add complete route-protection example to better-auth-plugins (#660 #12) A real Fresh routes/*.tsx handler gated on an active session and an admin role, at sibling auth.md's depth: resolves the session via backend.sessions.getSession, maps to a Principal with principalMapper, and fails closed on both checks. Grounded in packages/auth-better-auth + @netscript/service/auth exports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM * chore(run): docs-660b sweep worklog (#660) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VifzAytcxBk5odEMaCXihM --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Wave 2b — data adapters (A2) — IMPLEMENTATION (draft)
Sub-wave 2b of Wave 2. Targets the Wave 2 integration branch
feat/package-quality-wave2-adapters(umbrella #11), not the track directly — so the track only receives Wave 2 when the whole wave is complete.Run ID (to create):
feat-package-quality-wave2-adapters-2b--dataBase:
feat/package-quality-wave2-adapters@e5d54e2(base-synced — includes 2a: logger/telemetry/aspire)Depends on: 2a (merged) —
kvbuilds on@netscript/logger.Units (3, dependency order)
kv → database → prisma-adapter-mysqlScope highlights (from the Wave 2 plan, 23 slices)
kvbridges/intoadapters/(F-16);core/review; add/docs; README ≥150;ARCHITECTURE.md;./testingport-contract entrypointdatabase/docs+ tests;interfaces/→ ports (AP-17); the known slow-type fileprisma-adapter-mysqlskipLibCheck:true;examples/handling;/docsDefinition of Done (per unit)
deno publish --dry-run→ 0 slow types (no--allow-slow-types)deno doc --lintclean on everyexportsentrypoint/docsper STANDARDS §7archetype-gate-matrix.md)./testingport-contract entrypoint presentPASSdeno task e2e:cligreen + clean runtime (docker ps/aspire psempty)Process
PLAN-EVAL already
PASSfor the combined Wave 2 plan (Option A) — 2b does not re-run PLAN-EVAL; it implements the locked 2b slices and runs its own IMPL-EVAL in a separate session before this PR is marked ready.🤖 Generated with Claude Code